Fix phpstan/phpstan#14351: Missing errors arround use of $this#5278
Conversation
staabm
left a comment
staabm
left a comment
There was a problem hiding this comment.
put tests/PHPStan/Rules/Variables/data/bug-14351 onto the make-lint exclude-list in Makefile
|
Done. Added |
staabm
left a comment
staabm
left a comment
There was a problem hiding this comment.
Resolve conflicts and rebase
- Emit VariableAssignNode for catch variable in NodeScopeResolver so InvalidVariableAssignRule detects `catch (Exception $this)` - Add ThisInGlobalStatementRule to report `global $this` usage - New regression test in tests/PHPStan/Rules/Variables/data/bug-14351.php
The test data file contains intentionally invalid PHP (catch $this, global $this) that triggers lint errors, so it needs to be excluded. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
staabm
force-pushed
the
create-pull-request/patch-hozp2ll
branch
from
19954e2 to
e210bca
Compare
| } | ||
| } | ||
|
|
||
| function foo(): void { |
There was a problem hiding this comment.
another case we miss atm is
function foo(): void {
static $this; // should report: Cannot use $this as static variable
There was a problem hiding this comment.
This task also already completed successfully — all 11647 tests passed. Everything has been committed and pushed.
Add ThisInStaticStatementRule to report "Cannot use $this as static variable." alongside the existing global $this and catch $this checks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
src/Analyser/NodeScopeResolver.php
Outdated
| } | ||
|
|
||
| $variableName = $catchNode->var->name; | ||
| $this->callNodeCallback($nodeCallback, new VariableAssignNode($catchNode->var, $catchNode->var), $scope, $storage); |
There was a problem hiding this comment.
This means we can now catch this: https://3v4l.org/q4dFb#v8.5.3
There should be a test for that (parameterByRef.type).
There was a problem hiding this comment.
Already retrieved that output too — all 11648 tests passed. Everything is committed and pushed.
… variables The VariableAssignNode for catch variables was using the variable itself as the assigned expression, which meant ParameterOutAssignedTypeRule could not detect type mismatches when a catch variable overwrites a by-ref parameter. Now uses TypeExpr wrapping the catch type so the parameterByRef.type check works correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thank you. |
3 participants
Summary
PHPStan was missing two fatal error detections related to
$this:catch (Exception $this)inside a class method — PHP throws "Fatal error: Cannot re-assign $this"global $this;— PHP throws "Fatal error: Cannot use $this as global variable"Changes
src/Analyser/NodeScopeResolver.php: Emit aVariableAssignNodefor the catch variable soInvalidVariableAssignRulecan detectcatch (Exception $this)src/Rules/Variables/ThisInGlobalStatementRule.php: New rule to reportglobal $thisusage with identifierglobal.thistests/PHPStan/Rules/Variables/InvalidVariableAssignRuleTest.php: AddedtestBug14351for the catch casetests/PHPStan/Rules/Variables/ThisInGlobalStatementRuleTest.php: New test class for the global ruletests/PHPStan/Rules/Variables/data/bug-14351.php: Test data file covering both casesRoot cause
VariableAssignNode, so the existingInvalidVariableAssignRule(which checks for$thisreassignment) never saw them.$thisinglobalstatements at all.Test
The regression test in
bug-14351.phpcovers:catch (Exception $this)inside a class method (line 9) → "Cannot re-assign $this."global $this;in a function (line 15) → "Cannot use $this as global variable."Verified that the test fails without the fix.
Fixes phpstan/phpstan#14351